-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Display ordered NPC path on map, move TALK_GOTO_LOCATION to top-level menu #74093
Display ordered NPC path on map, move TALK_GOTO_LOCATION to top-level menu #74093
Conversation
eb061f7
to
33c8311
Compare
I'm going to put off changing their actual pathing until a future PR, since I haven't gotten around to it yet. Don't want to have this sit here forever waiting for me to finish something it doesn't need to wait on. |
33c8311
to
255a6a5
Compare
255a6a5
to
06b458d
Compare
+don't overrun our iterators
src/game.h
Outdated
@@ -1154,6 +1154,9 @@ class game | |||
// show NPC pathfinding on overmap ui | |||
bool debug_pathfinding = false; // NOLINT(cata-serialize) | |||
|
|||
//Ugly kludge to pass info to tile overmaps | |||
npc *follower_path_to_show = nullptr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really don't like this but the only alternative I see is passing display_path into a long chain of functions to get it to show up for tile overmaps.
@@ -689,6 +690,11 @@ static void draw_ascii( | |||
npc *npc_to_add = npc_to_get.get(); | |||
followers.push_back( npc_to_add ); | |||
} | |||
if( !display_path.empty() ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because this is technically passed different data than the tile overmaps, future changes could accidentally knock them out of sync.
It would be ideal to NOT have these be different, but the kludge in game.h is not something I want to rely on. That is why they are still different. But having them be different at all is another problem...
2b43a7c
to
0d6eb29
Compare
Are you waiting on reviews for this or is this ready for merge? |
It'd be nice if someone had a better way to handle the stuff I've commented on, but otherwise it's good to go. Not a hard requirement, IMO |
Oops, I forgot to edit the changelog line. Uh, hopefully one of us can catch that in the weekly changelog before it gets merged |
Summary
Balance "Display ordered NPC path on map, NPCs refuse to go to dangerous locations "
Purpose of change
NPC followers are
suicidally loyal andhard to keep track ofCamp ( 312, 555, 0)
where the heck is that? Dang it I knew I shouldn't have named all of my campsCamp
.Describe the solution
-Display the route automatically at the time of ordering the movement.
--The map automatically pans over the route and pauses at the end
-Move the "TALK_GOTO_LOCATION" topic to the top-level menu, instead of burying it 2 menus deep in a camp topic(?!)
-Display an estimated time of arrival and a query so you know you sent the NPC to the right place
Describe alternatives you've considered
Testing
2024-06-16.08-24-44.mp4
Additional context
Plans for pathfinding changes were pushed out to future PRs, see edit record/followup message
Known issues:
The highlighted red path sometimes does not display on the opened map, depending on when the last blink was shown.
(It will still step through each tile as appropriately, it just won't have a visible trail it's following)
The speed at which it advances through the tiles is rather slow, limited by how often the frames are redrawn rather than the actual code limitations I've applied.
--Moving the mouse causes it to go very fast, because mouse inputs trigger new frames to be drawn.
--Regardless, you can exit the overmap ui at any time, so you don't have to wait to see. But you can.